Pims fallback for avi loading #1161

EricThomson · 2023-08-24T21:00:24Z

Description

This should fix the problems with loading avi in Windows. It cleans up the logic of the pims fallback branch (thanks Pat), and I worked on the subindixing bits in pims, which was also neglected. Main change is fix to caiman_.base.movies.load() and .load_iter() methods (the later is used in online methods)

Fixes #1137 #883

Type of change

Bug fix (non-breaking change which fixes an issue)

Introduces no new problems with tests.

Ran tests in pims_load_testing.py

Now the cnmfe online demo notebook runs (demo_online_cnmfE.ipynb). It would be good if someone tests on linux/mac.

…for avi files) to the one that load() has

…separate function so we don't repeat it

… CAP_DSHOW in opencv - doesn't actually work as a workaround

… does

…ecked

Dev load pims fallback

…xtra sanity-checks. Remove some more tabs.

pgunn

generally looks good. Solid debugging/fixup work!

pgunn · 2023-08-24T22:08:45Z

caiman/base/movies.py

@@ -1331,7 +1331,7 @@ def load(file_name: Union[str, List[str]],
                                is3D=is3D)

    elif isinstance(file_name, tuple):
-        print(f'**** Processing input file {file_name} as individualframes *****')
+        logging.debug(f'**** Processing input file {file_name} as individualframes *****')


Might be a good time to stick a space between individual and frames

pgunn · 2023-08-24T22:09:16Z

caiman/base/movies.py

-            # Instead we'll explicitly go with the DirectShow backend on Windows, which
-            # seems to have good broad codec support (if this turns out to break other users,
-            # we may need a more complex solution)
+            # We first try with OpenCV, which is very dependent on having a working backend.


Did I stick the word "very" in there? If I did, my bad. Shouldn't be there.

pgunn · 2023-08-24T22:10:12Z

caiman/base/movies.py

-                        dims[ind] = sb.shape[0]
-
-                start_frame = subindices[0][0]
+            if length <= 0 or width <= 0 or height <= 0:       # OpenCV failure


No need to do it here (not really a comment for review, per se) but I eventually intend to pull this check out into a separate function because we do it a few times in a few places

pgunn · 2023-08-24T22:10:35Z

caiman/base/movies.py

-                def rgb2gray(rgb):
-                    return np.dot(rgb[..., :3], [0.299, 0.587, 0.114])
-
+                # When everything done, release the capture


When everything is done

pgunn · 2023-08-24T22:11:00Z

caiman/base/movies.py

+                input_arr = np.zeros((dims[0], height, width), dtype=np.uint8)
+                for i, ind in enumerate(subindices[0]):
+                    input_arr[i] = rgb2gray(pims_movie[ind]).astype(outtype)
+                    # logging.debug(f"subindex and shape: {ind} {input_arr[i].shape}") # helpful when mc debugging deep in weeds, otherwise overkill 


remove before commit?

pgunn · 2023-08-24T22:13:06Z

caiman/base/movies.py

@@ -1982,7 +2026,7 @@ def from_zip_file_to_movie(zipfile_name: str, start_end: Tuple = None) -> Any:
                        mov[counter] = np.array(Image.open(file))

                    if counter % 100 == 0:
-                        print([counter, idx])
+                        logging.debug([counter, idx])


Let's clean this up a bit (not your fault, but since we're touching it); passing raw arrays to printing function means they're going to show up raw and ugly.

pgunn · 2023-08-24T22:14:37Z

caiman/base/movies.py

+                                        1 if subindices.step is None else subindices.step)
+                                t = 0
+                                for ind in subindices:
+                                    # an abomination reading frames until it hits the desired frame


not sure we want to use this wording

pgunn · 2023-08-24T22:15:12Z

caiman/base/movies.py

+                                               1 if subindices.step is None else subindices.step)
+                        for ind in subindices:
+                            frame = rgb2gray(pims_movie[ind])
+                            # debug.logging(f"t, ind and frame shape: {ind} - {frame.shape}") # for extremely in the weeds debugging 


Do we need to leave this in?

pgunn · 2023-08-24T22:16:48Z

demos/notebooks/demo_online_cnmfE.ipynb

@@ -78,7 +107,7 @@
   "source": [
    "# motion correction parameters\n",
    "motion_correct = True            # flag for performing motion correction\n",
-    "pw_rigid = False                 # flag for performing piecewise-rigid motion correction (otherwise just rigid)\n",
+    "pw_rigid = True                  # flag for performing piecewise-rigid motion correction (otherwise just rigid)\n",


Did you mean to change this (and in this diff) or was this just for debugging??

Good eye that was for debugging. I'll switch it back

pgunn · 2023-08-24T22:17:20Z

demos/notebooks/demo_online_cnmfE.ipynb

@@ -336,7 +391,7 @@
    "gSig = (10//ds_factor, 10//ds_factor)          # expected half size of neurons\n",
    "gSiz = (22//ds_factor, 22//ds_factor)\n",
    "sniper_mode = False                            # flag using a CNN to detect new neurons (o/w space correlation is used)\n",
-    "init_batch = 300                               # number of frames for initialization (presumably from the first file)\n",
+    "init_batch = 200                               # number of frames for initialization (presumably from the first file)\n",


same with these other notebook changes

pgunn · 2023-08-25T05:05:40Z

demos/notebooks/demo_online_cnmfE.ipynb

@@ -291,7 +348,7 @@
   "metadata": {},
   "outputs": [],
   "source": [
-    "cnm.estimates.play_movie(images, magnification=0.75, include_bck=False)"
+    "cnm.estimates.play_movie(images, magnification=0.75, include_bck=False);"


semicolons are a bit unusual in python

pgunn · 2023-08-25T19:39:43Z

caiman/motion_correction.py

@@ -3076,6 +3076,9 @@ def tile_and_correct_wrapper(params):
    extension = extension.lower()
    shift_info = []

+    print("In tile and correct wrapper about to call load with subidxs")


These messages are noisy when run in the notebook

Oops. Cutting errant debugging msg.

pgunn · 2023-08-25T19:43:46Z

demos/notebooks/demo_online_cnmfE.ipynb

+    "                                                         magnification=1,\n",
+    "                                                         gain=0.6,\n",
+    "                                                         plot_text=True,\n",
+    "                                                         do_loop=True)\n",


if we're going to default to looping, we should tell the user that they shouldn't keep waiting

Good point I'll cut that.

pgunn · 2023-08-25T19:45:27Z

demos/notebooks/demo_online_cnmfE.ipynb

+   "cell_type": "markdown",
+   "metadata": {},
+   "source": [
+    "Inspect the summary images so you can set parameters in the following section."


This cell was not commited pre-run?

was not executed, I mean

pgunn · 2023-08-25T19:47:19Z

demos/notebooks/demo_online_cnmfE.ipynb

@@ -251,6 +301,15 @@
    "t1 += time()"


This is more clever than it is legible

pgunn and others added 15 commits August 4, 2023 15:31

movies.py: load_iter(): Build out a similar pims fallback code path (…

7458d55

…for avi files) to the one that load() has

load_chain(): fix avi loader logic. Also move rgb2gray() inline into …

09e3745

…separate function so we don't repeat it

source_extraction/cnmf/utilities.py:get_file_size(): don't try to use…

57130a1

… CAP_DSHOW in opencv - doesn't actually work as a workaround

environment.yml and environment-minimal.yml: add moviepy (for pims)

0cc933e

Have length calculation code also fall back to pims like loading code…

e4d470a

… does

updated pim fallback to all nonpositive vals <=0

24cfc88

some funky indentation issues in movies.py fixed. might need to be ch…

534a1c8

…ecked

added movie viewer to be sure load works

003e875

added loggers and notes about subindices potential bug

55b9b68

Merge pull request #1147 from EricThomson/dev_load_pims_fallback

55cb32d

Dev load pims fallback

base/movies.py:load() - backport improvements from load_iter(). Add e…

9865694

…xtra sanity-checks. Remove some more tabs.

base/movies.py: fix some formatting

3588a63

subinds problem fixed, minor update to online cnmfe demo

fa40755

took out some debugging prints from motioncorrection.py

3f75c0b

pims fallback for load/loaditer should work with/without subindices

5ca1184

EricThomson requested review from pgunn and kushalkolar August 24, 2023 21:01

pgunn reviewed Aug 24, 2023

View reviewed changes

updated based on review

e8aaef2

pgunn reviewed Aug 25, 2023

View reviewed changes

demos/notebooks/demo_online_cnmfE.ipynb Outdated

@@ -251,6 +301,15 @@

"t1 += time()"

Copy link

Member

pgunn Aug 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is more clever than it is legible

updated from review bits

e7055c0

pgunn merged commit f3326c2 into dev Aug 26, 2023

EricThomson mentioned this pull request Oct 15, 2023

Movie save for avi in Windows doesn't work #1192

Open

pgunn deleted the dev_load_pims_fallback branch March 5, 2024 20:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pims fallback for avi loading #1161

Pims fallback for avi loading #1161

EricThomson commented Aug 24, 2023 •

edited

Loading

pgunn left a comment

pgunn Aug 24, 2023

pgunn Aug 24, 2023

pgunn Aug 24, 2023

pgunn Aug 24, 2023

pgunn Aug 24, 2023

pgunn Aug 24, 2023

pgunn Aug 24, 2023

pgunn Aug 24, 2023

pgunn Aug 24, 2023

EricThomson Aug 25, 2023

pgunn Aug 24, 2023

pgunn Aug 25, 2023

pgunn Aug 25, 2023

EricThomson Aug 25, 2023

pgunn Aug 25, 2023

EricThomson Aug 25, 2023

pgunn Aug 25, 2023

pgunn Aug 25, 2023

pgunn Aug 25, 2023

Pims fallback for avi loading #1161

Pims fallback for avi loading #1161

Conversation

EricThomson commented Aug 24, 2023 • edited Loading

Description

Type of change

pgunn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

EricThomson commented Aug 24, 2023 •

edited

Loading